Skip to content

Conversation

@Proxy-99
Copy link
Contributor

Add validation
#4039

@Proxy-99 Proxy-99 requested a review from Lpsd February 21, 2025 17:48
Lpsd
Lpsd previously approved these changes Feb 22, 2025
@Lpsd Lpsd dismissed their stale review February 22, 2025 19:47

Issue noticed

Copy link
Member

@Lpsd Lpsd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please resolve this using the suggestion in the previous review. There is a reason the MAX entries are defined in the enum (also the reason why I made that suggestion).

Whilst it's unlikely the enum will change, if new entries are added then your code would be faulty and need updating. We would rather avoid that entirely (making less work for future contributors), which is why you compare to the MAX enum entry for such range checks.

@Proxy-99
Copy link
Contributor Author

Please resolve this using the suggestion in the previous review. There is a reason the MAX entries are defined in the enum (also the reason why I made that suggestion).

Whilst it's unlikely the enum will change, if new entries are added then your code would be faulty and need updating. We would rather avoid that entirely (making less work for future contributors), which is why you compare to the MAX enum entry for such range checks.

done

TheNormalnij
TheNormalnij previously approved these changes Feb 23, 2025
@Lpsd
Copy link
Member

Lpsd commented Feb 23, 2025

Looks good now, thanks!

@botder botder merged commit c1e6be1 into multitheftauto:master Feb 23, 2025
6 checks passed
@botder botder added this to the 1.6.1 milestone Feb 23, 2025
@botder botder added the bugfix Solution to a bug of any kind label Feb 23, 2025
MTABot pushed a commit that referenced this pull request Feb 23, 2025
@Proxy-99 Proxy-99 deleted the fix-bind branch September 2, 2025 18:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix Solution to a bug of any kind

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants